-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce XCQ #126
base: main
Are you sure you want to change the base?
Introduce XCQ #126
Conversation
0af3db3
to
220f62d
Compare
```rust | ||
decl_runtime_apis! { | ||
pub trait XcqApi { | ||
fn execute_query(query: Vec<u8>, input: Vec<u8>, weight_limit: u64) -> XcqResult; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess any user can call this Runtime API and provide their XCQ program and input args? Is it expected that the runtime will have some default gas/weight limit to make it more difficult for users to supply long running/complex programs that DoS the node?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The weight_limit
argument in this single API restricts the usage of a single execution. The overall resource limit is enforced by Runtime config and RPC rate limiting, which is out of the scope for a single RuntimeAPI. As for XCM usage, the gas/weight limit can be enforced by BuyExecution
instruction.
```rust | ||
decl_runtime_apis! { | ||
pub trait XcqApi { | ||
fn execute_query(query: Vec<u8>, input: Vec<u8>, weight_limit: u64) -> XcqResult; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the weight limit one-dimensional when two-dimensional weights are in use now? Is this weight somehow different? Or is it depends on the context? If so, can we use a different word to avoid confusion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proof size doesn't really matter in the context of runtime API but yeah u64 is ambitious. We could use the Substrate Weight type and ignore proof size
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could name it weight_ref_time_limit
to emphasize that it is intentionally one-dimensional. We could also leave a comment reminding people that the proof size isn't relevant in this context.
impl extension_core::ExtensionCore for ExtensionImpl { | ||
type Config = ExtensionImpl; | ||
fn has_extension(id: <Self::Config as extension_core::Config>::ExtensionId) -> bool { | ||
matches!(id, 0 | 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought extension ids were hashes of the content? The extension decides its id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I read the RFC, I understood the ExtensionCore
as a "special" "foundational" extension. It seems it will have some generic implementation where the extension IDs will indeed be content-addressed.
Is that so? Is this code just an example of "some return value"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is just a dummy implementation. we should leave it out in the example
text/0126-introduce-xcq.md
Outdated
```rust | ||
ReportQuery { | ||
query: SizeLimitedXcq, | ||
weight_limit: Option<Weight>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can someone get this weight? Is it easy? If not, we could just limit it by the overall fee paid in PayFees
?
We need to have a good way of using this, otherwise everyone will just use Unlimited
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should modify execute_query
to have it also return used weight so it can be used to estimate weight usage
We could make Unlimited
weight means the weight to be limited by fees. Limited weight will be useful for the case that we need to reserve some fees for other instruction, such as transact
`ErrorCode` is a enum | ||
- `ExceedMaxWeight = 0` | ||
- `MemoryAllocationError = 1` | ||
- `MemoryAccessError = 2` | ||
- `CallError = 3` | ||
- `OtherPVMError = 4` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if there's a way to improve these errors. I guess you could run and debug your XCQ program directly on the destination chain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes everything are deterministic so replay with extra logging enabled should be the preferred way for debugging. That's how we debug XCM today.
Looking for feedback.
Summary
This proposal introduces XCQ (Cross Consensus Query), which aims to serve as an intermediary layer between different chain runtime implementations and tools/UIs, to provide a unified interface for cross-chain queries.
Related RFC
A related XCM-format RFC is drafting.
Related Discussions
https://forum.polkadot.network/t/wasm-view-functions/1045
PoC implementations
https://github.com/open-web3-stack/XCQ